Skip to content

Time offset fix - #43

Merged
rcjackson merged 3 commits into
ARM-Development:mainfrom
rcjackson:time_offset_fix
Sep 12, 2026
Merged

Time offset fix#43
rcjackson merged 3 commits into
ARM-Development:mainfrom
rcjackson:time_offset_fix

Conversation

@rcjackson

Copy link
Copy Markdown
Collaborator

Fix time_offset from being dropped from the 90 degree PPI retrieval.

Robert Jackson and others added 3 commits September 12, 2026 06:46
A site the radar never sampled comes back from Py-ART as an all-NaN
column. column_vertical_profile averages an empty set of rays, so
time_offset -- the mean gate time -- is NaN right along with every
field. subset_points purged every all-NaN variable and took
time_offset with it, then raised KeyError: 'time_offset' three lines
later at the gate_time calculation. np.isnan returns True for NaT, so
the check fired on the time columns as readily as on the float ones.

The whole scan was lost, and every co-located site in it, not just the
one out of coverage.

time_offset is bookkeeping, not a measurement. It is now exempt from
the purge and from the dropna height mask, so an out-of-coverage
column comes back whole with time_offset and gate_time as NaT --
missing, which is true.

The dropna exemption also fixes a second, quieter loss: a partly-NaN
time_offset stayed in the dataset and helped decide which heights
survived, discarding gates the radar had genuinely sampled.

Two more in the same block:

- da.drop(v) -> da.drop_vars(v). The former is deprecated and emitted
  a FutureWarning on every all-NaN field.
- Guard the time_offset.drop_duplicates call in the InvalidIndexError
  handler behind the RHI check. time_offset is only bound in the RHI
  branch, so a non-RHI column with duplicate heights hit NameError, or
  silently reused the previous site's offsets.

This predates the pluvio accumulation fix and is independent of it;
the all-NaN purge came in with d837757 (ADD: XSAPR). Scans dropped to
this KeyError were skipped rather than corrupted, so no written output
needs revisiting on its account.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@rcjackson
rcjackson merged commit a649598 into ARM-Development:main Sep 12, 2026
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant